New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Axis Labels with offset Spines #4134
Conversation
Should probably add a test for this as well |
@@ -1743,7 +1745,9 @@ def _update_label_position(self, bboxes, bboxes2): | |||
|
|||
else: | |||
if not len(bboxes2): | |||
top = self.axes.bbox.ymax | |||
top = (self.axes.spines['top'].get_transform(). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docstring of the function only talks about tick labels, now the function uses spines too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. Updated the docstrings
""" | ||
if not self._autolabelpos: | ||
return | ||
x, y = self.label.get_position() | ||
if self.label_position == 'bottom': | ||
if not len(bboxes): | ||
bottom = self.axes.bbox.ymin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make more sense to just throw the bounding box of the spine and the axes in to the bboxes list and let the union
call take care of this logic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, @tacaswell, I think that makes more sense. I'll update to do just that.
… all ticklabels and axis spine for use in label placement
else: | ||
bbox = mtransforms.Bbox.union(bboxes) | ||
bottom = bbox.y0 | ||
spinebbox = (self.axes.spines['bottom'].get_transform(). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style alternative: I would factor out spine = self.axes.spines['bottom']
so as to make the spinebbox expression more compact.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that is much better. Will modify it as so
Looks good. A test would be nice. |
Working on a test |
…no tick labels are present and axis spines are offset (see issue matplotlib#3715)
…ng pep8 now. Modified test_spines to not use assert_less since it is not available in python 2.6
BUG : Axis Labels with offset Spines Closes #3715
Thank you |
An attempt to fix the issue #3715. This takes into account the location of the axis spines when locating the axis label if there are no tick labels present.
Examples in #3715
Before patch
After patch